Use EventPriority to track update priority - #21082
Conversation
facebook-github-bot
added
React Core Team
|
|
||
| let currentUpdatePriority: EventPriority = NoLane; | ||
|
|
||
| export function getCurrentUpdatePriority(): EventPriority { |
There was a problem hiding this comment.
Re: naming, I was thinking I would rename this to getCurrentEventPriority, and then rename the host config one to something like getCurrentHostEventPriority. Gonna leave as-is in this PR, though.
There was a problem hiding this comment.
Why bother to rename more than this if we're going to remove it?
There was a problem hiding this comment.
This is staying. It tracks a Lane now instead of a LanePriority.
|
Comparing: 148f8e4...4b7ff7f Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
35f87aa to
64284ec
Compare
Instead of LanePriority. Internally, EventPriority is just a lane, so this skips an extra conversion. Since EventPriority is a "public" (to the host config) type, I was also able to remove some deep imports of the Lane module. This gets us most of the way to deleting the LanePriority entirely.
64284ec to
4b7ff7f
Compare
| // Copied from ReactFiberLanes. Don't do this! | ||
| // This is hard coded directly to avoid needing to import, and | ||
| // we'll remove this as we replace runWithPriority with React APIs. | ||
| export const IdleLanePriority = 2; |
| blockedOn: null, | ||
| target: target, | ||
| lanePriority: updateLanePriority, | ||
| priority: updatePriority, |
There was a problem hiding this comment.
It comes full circle - this was the original name of the field when it was the scheduler priority, which we had to keep alongside lanePriority.
| }; | ||
| let i = 0; | ||
| for (; i < queuedExplicitHydrationTargets.length; i++) { | ||
| // Stop once we hit the first target with lower priority than |
| !isHigherEventPriority( | ||
| updatePriority, | ||
| queuedExplicitHydrationTargets[i].priority, | ||
| ) |
| ? (IdleEventPriority_new: any) | ||
| : (IdleEventPriority_old: any); | ||
|
|
||
| export function runWithPriority<T>(priority: EventPriority, fn: () => T): T { |
There was a problem hiding this comment.
I think this is only for tests now - how long until we can remove this? We'll need Offscreen for the low priority, but can we replace the Continuous with something that's actually continuous instead?
|
|
||
| let currentUpdatePriority: EventPriority = NoLane; | ||
|
|
||
| export function getCurrentUpdatePriority(): EventPriority { |
There was a problem hiding this comment.
Why bother to rename more than this if we're going to remove it?
| finishedRoot: FiberRoot, | ||
| current: Fiber, | ||
| nearestMountedAncestor: Fiber, | ||
| renderPriorityLevel: LanePriority, |
There was a problem hiding this comment.
Nice find, I noticed this was unused too.
| const schedulerPriority = | ||
| priorityLevel === NoLanePriority | ||
| ? NormalPriority | ||
| : lanePriorityToSchedulerPriority(priorityLevel); |
There was a problem hiding this comment.
Why inline? Do you anticipate this is the only place you'll need to do this conversion?
There was a problem hiding this comment.
Currently it is, yeah. Also there's a backlog item to discuss what kind of priority to expose and how. Scheduler priority doesn't really make sense anymore because of our decoupling work.
So inlining makes it harder for it to leak to more places.
|
|
||
| export const DiscreteEventPriority: EventPriority = enableNewReconciler | ||
| ? (DiscreteEventPriority_new: any) | ||
| : (DiscreteEventPriority_old: any); |
There was a problem hiding this comment.
The host config isn't forked, but it handles types from both reconciler forks. Like Fiber is technically a cross-fork type, because it gets passed between the host config and reconciler. That's why we have the ReactInternalTypes module.
Other than forking all the renderers, there's no way (at least that I've that I've found) to express that the type passed is the correct one for the given fork. So we cheat.
5 participants
Instead of LanePriority. Internally, EventPriority is just a lane, so this skips an extra conversion. Since EventPriority is a "public" (to the host config) type, I was also able to remove some deep imports of the Lane module.
This gets us most of the way to deleting the LanePriority entirely.